Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update: hr within select element #504

Merged
merged 4 commits into from
Oct 9, 2023
Merged

Update: hr within select element #504

merged 4 commits into from
Oct 9, 2023

Conversation

scottaohara
Copy link
Member

@scottaohara scottaohara commented Oct 3, 2023

closes #480

Outside of browsers / AT significantly changing how people interact with the UA-provided listboxs, I don't see how the use of hr can be more than declarative decoration, since the element will not be directly accessible to anyone using AT.

If anyone thinks otherwise, and browsers/AT are willing to make changes to how this component has been behaving/exposed, we can discuss and change the PR.


Preview | Diff

closes #480 

Outside of browsers / AT significantly changing how people interact with the UA-provided listboxs, I don't see how the use of hr can be more than declarative decoration, since the element will not be directly accessible to anyone using AT.

If anyone thinks otherwise, and browsers/AT are willing to make changes to how this component has been behaving/exposed, we can discuss and change the PR.
@@ -2792,6 +2792,7 @@ <h4 id=el-hr>`hr`</h4>
<tr>
<th>Comments</th>
<td>
<p>If an `hr` element is a descendant of a `select` element, user agents MAY expose the element with a role of `none`.</p>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This matches the standard list separator behavior in Mac, and it's also a MAY, so probably okay to approve without requiring input from Chrome, Edge, and Gecko, but as a courtesy, we should request input from @aleventhal @benbeaudry and @jcsteh

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For clarity, my @ mention above counts as that courtesy request. @scottaohara if you don't hear back in a week and have a second approval, please merge.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me. +1

@scottaohara
Copy link
Member Author

Looks like this is shipping in chrome 119 - https://bugs.chromium.org/p/chromium/issues/detail?id=1441883. Nothing special going on with that implementation for the hr elements. they're just rendering some visual separators with no exposure to the a11y tree, from what I can tell (which is good, imo).

Firefox bug as well: https://bugzilla.mozilla.org/show_bug.cgi?id=1830909

going to merge this per the webkit/upcoming chrome implementations

@scottaohara scottaohara merged commit 1dc2dba into gh-pages Oct 9, 2023
1 check passed
@scottaohara scottaohara deleted the hr-select branch October 9, 2023 19:22
@annevk
Copy link
Member

annevk commented Oct 10, 2023

Wouldn't it make more sense to continue map it to separator and then let the decision of what to do with separator remain there? At least for Apple platforms we wouldn't have specific code for <hr>-in-<select> vs outside.

@cookiecrook
Copy link
Collaborator

cookiecrook commented Oct 10, 2023

Wouldn't it make more sense to continue map it to separator and then let the decision of what to do with separator remain there? At least for Apple platforms we wouldn't have specific code for <hr>-in-<select> vs outside.

As I see it, there are 3 options:

  1. If this role (separator in ~listbox) was exposed all the way to the platform API, there's a possibly it could cause downstream bugs in Assistive Technologies… or would put the onus on all of those AT projects to account for a web-specific mapping diff. All engines ship on multiple platforms, and every platform has multiple ATs, so this option would require the most amount of effort to maintain. So I'd prefer to keep any change in the engine.

  2. Within the engine, the change could be in the internal accessibility tree (as this MAY allows/suggests) or in the platform-specific mappings, which are specified in HTML-AAM and Core-AAM, but those are more-or-less evergreen specs intended to match the platform API owners discretion. If the change were in the platform mappings, it would still increase code complexity: different mapping rules times however many platforms the engine ships on, times however many platform-specific Accessibility APIs the platform has (Windows has multiple).

  3. The final option only requires one change per engine, in the role implementation. The platform mappings and AT are all downstream, so this would be easiest to maintain.

So I would still prefer to let the spec text remain as above... It's a MAY after all, but an educated MAY... Effectively ~"Implementors, if you need to change something, change it here, not there. Don't toss it over the fence and make the AT do all the work."

@MattWilcox
Copy link

MattWilcox commented Oct 24, 2023

I fail to see why this is a thing at all; what's the use case/justification for this <hr> behaviour here?

What is a separator if not a grouping element. By separating things you are grouping them; it's one and the same thing. All I see happening here is that a generation of people already not aware of the <optgroup> tags will not go looking and we'll have "grouping" done by non-semantic HR tags which can't be labeled. Which, I'm sure, will lead to this sort of garbage later:

<select>
<option disabled>Section 1</option>
<option value=1>One</option>
<option value=2>Two</option>
<hr>
<option disabled>Section 2</option>
<option value=2-1>One</option>
<option value=2-2>Two</option>
</select>

This seems like a solution to a styling problem that isn't actually a problem, and is going to make more problems for more people, IMO.

@scottaohara
Copy link
Member Author

hi @MattWilcox, might i suggest you post your comment over in the HTML issue/PR that added this allowance? For instance, whatwg/html#9124

@annevk
Copy link
Member

annevk commented Oct 24, 2023

It's an unnamed optgroup, essentially: whatwg/html#3410 (comment). It's a fairly common UI pattern.

@jcsteh
Copy link

jcsteh commented Oct 24, 2023

Outside of browsers / AT significantly changing how people interact with the UA-provided listboxs, I don't see how the use of hr can be more than declarative decoration, since the element will not be directly accessible to anyone using AT.

This is not necessarily true:

  1. A user could use a screen reader's review cursor (e.g. NVDA object navigation) to locate the separators. In theory, the VoiceOver cursor could work too, except that I gather MacOS explicitly doesn't expose separators in lists.
  2. For a long time now, Firefox has calculated group position information (implicit posinset and setsize) accounting for separators. For example, if you have a menu containing 6 items with a separator between items 3 and 4, focusing the first item would report "1 of 3" with a screen reader, as would focusing the 4th item. Strictly speaking, this is a spec violation, though this behaviour was implemented long before the currently specified behaviour. While this is not a perfect solution - some users can be confused about why the item count starts again - it does allow a user to perceive the existence of the separators without making them focusable or requiring the use of things like screen reader review cursors.

IMO, it's not great that we're effectively saying that we "might not" make the existence of separators perceivable to some users. If they're present visually, that suggests there is a good reason for that.

Copy link
Collaborator

@cookiecrook cookiecrook left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jcsteh wrote:

IMO, it's not great that we're effectively saying that we "might not" make the existence of separators perceivable to some users.

I assume you'd prefer this is exposed in some predictable way (role=separator) that the AT can choose to ignore or present. Is that your intention? If so, the only resolution you'd be happy with is rejecting this PR in its entirety, correct?

@jcsteh
Copy link

jcsteh commented Oct 25, 2023

I'm mindful that this is not a straightforward situation. While not ideal IMO, if MacOS has not exposed separators in native lists forever and doing so will break things horrendously, then obviously this is going to be a big lift to fix. All of us have to pick the battles on which we are willing to expend the most energy. Given that we almost certainly have bigger fish to fry across the board, I don't think this is one of those cases for me. That said, I thought it was important to point out that hr can absolutely be (and possibly should be) more than declarative decoration on some platforms. This PR doesn't preclude that, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

<hr>-in-<select> semantics
7 participants